[v0.2.0 Refactor] [Type-C] Remove static_cell dependency and improve context handling#668
Conversation
8c975f6 to
16aadd1
Compare
asasine
left a comment
There was a problem hiding this comment.
Looks functional so approved. All of my comments are therefore non-blocking.
&'static->&'ais a step for testability.- Adding methods to
Contextand getting rid of the external public static functions is a usability improvement. - Using
self.controllers()instead of passing a field as an arg to your own methods is hygeine and usability.
4064a52
4064a52 to
113f3af
Compare
…context handling - Removed `static_cell` dependency from Cargo.lock and Cargo.toml in examples and type-c-service. - Updated Type-C service methods to accept a reference to `intrusive_list::IntrusiveList` for better context management. - Modified various service methods to pass the controllers list where necessary, ensuring proper context usage. - Commented out unused code related to power policy channels and service initialization. - Adjusted the task closure to work with the updated service structure. - Enhanced error handling and logging throughout the service methods. - Initializing the service will now register all PD controllers before running the type-c task WIP
113f3af to
fb4a619
Compare
There was a problem hiding this comment.
Exposing intrusive_list::IntrusiveList as a part of the public API worries me a little bit as it is a bit not user friendly as an API. But if all the code owners for type C agrees that this is the most optimal way to do it, then it is fine.
@tullom Can you post this PR with a high-level description of why we are doing this refactoring on Zulip or maybe as a RFC on https://github.com/OpenDevicePartnership/governance/tree/main/rfc? Thanks.
Long-term I think there's a good chance we'll end up switching to slices or something similar. But I think this is a good incremental change towards that goal. |
Done, see Zulip thread here: |
static_celldependency from Cargo.lock and Cargo.toml in examples and type-c-service.intrusive_list::IntrusiveListfor better context management.